Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow all directories to be "safe" in git terms #2100

Merged
merged 1 commit into from
Jul 8, 2024
Merged

allow all directories to be "safe" in git terms #2100

merged 1 commit into from
Jul 8, 2024

Conversation

evgeni
Copy link
Member

@evgeni evgeni commented Jul 8, 2024

Git in EL9+ only allows to clone repositories that one is the owner of.
This obviously doesn't work for shared repositories like we have them
for secretsgit.

Disable that feature alltogether on systems that serve as secretsgit
sources.

Sadly a more specific wildcard is not supported [1] and given Puppet
doesn't know which stores we have, I've opted to completely disabling
this feature.

[1] https://git-scm.com/docs/git-config/2.45.0#Documentation/git-config.txt-safedirectory

@evgeni evgeni mentioned this pull request Jul 8, 2024
@evgeni
Copy link
Member Author

evgeni commented Jul 8, 2024

CI fix in #2102

puppet/modules/secretsgit/manifests/init.pp Outdated Show resolved Hide resolved
@@ -0,0 +1,2 @@
[safe]
directory = *
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a template and we put $secretsgit::path in there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$secretgit::path is the base, and our repos are ${secretgit::path}/blah.git etc. You need to pass full paths to those repos here, or *, otherwise it won't work (I tried).

@@ -16,6 +16,14 @@
Stdlib::Absolutepath $path = '/srv/secretsgit',
Array[String] $users = [],
) {
file { '/etc/gitconfig':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The alternative would be to set this for each repo we have, but we don't manage those. Would that be preferable? Extend the creation instructions on line 7 with a repo config.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd mean every time you create a repo you need someone with root permissions to edit the system config, sounds annoying to me.
(but then again, we do not create new repos that often)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'd mean every time you create a repo you need someone with root permissions to edit the system config, sounds annoying to me.

Don't you already need that now?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't being in the secrets group be enough?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then shouldn't being in the secrets group be enough to run git --git-dir $REPO config safe.directory $REPO?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Safe directory is only accepted in system and global configs, otherwise a repo could declare itself safe

Git in EL9+ only allows to clone repositories that one is the owner of.
This obviously doesn't work for shared repositories like we have them
for secretsgit.

Disable that feature alltogether on systems that serve as secretsgit
sources.

Sadly a more specific wildcard is not supported [1] and given Puppet
doesn't know which stores we have, I've opted to completely disabling
this feature.

[1] https://git-scm.com/docs/git-config/2.45.0#Documentation/git-config.txt-safedirectory
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this isn't a regression compared to our EL8 host and it's not like we really use git elsewhere, so I'm fine with this. Perhaps our puppet config deploy, but I'm not too concerned with that now.

@ekohl ekohl merged commit 7ffbbaa into master Jul 8, 2024
2 checks passed
@ekohl ekohl deleted the secretsgit branch July 8, 2024 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants